fix(server): reject if error in search or recommend #6616
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit db675bc:
|
dhayab
left a comment
dhayab
left a comment
There was a problem hiding this comment.
Great refactoring. The only difference I see is that we now explicitly search again from within waitForResults() whereas before the search would have happened earlier. Don't think it makes a difference in the end.
|
I want to evaluate the feedback in #6525 a little more, it seems that if the search throws there's still some sort of (in)finite nesting of the error going on, although search seems to be called only once |
|
@Haroenv it works for me, I checked it on my current project. I used monkey-patched version before, but your solution looks better in error handling. |
packages/react-instantsearch-nextjs/src/__tests__/InitializePromise.test.tsx
Outdated
Show resolved
Hide resolved
|
Thanks @izorg, I tried rebasing but I believe something has changed in the implementation now and the tests I previously had don't pass anymore (I believe the widgets don't get mounted). For now I'd say you best focus on keeping the patch in place that you have, or if you find what I'm missing in these tests, let me know |
|
I am sorry @Haroenv, you are right, widgets are not mounted on server side. I tested the bad case with infinite loop, but did not pay enough attention on checking working cases. |
Summary
Refactors InitialisePromise to use the waitForResults of InstantSearch.js, which correctly rethrows errors if they happen in search, composition or recommend.
Be careful when refactoring to ensure I didn't misunderstand the intent of the original code, as I don't remember why we didn't use InstantSearch' waitForResults in the first place. There may be a subtle difference between the functions I didn't catch.
Result
fixes #6525